Skip to content

worker: Clone index repository in the background #7461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Nov 6, 2023

We are currently delaying the execution of any tasks until the index repository is cloned. This was useful while we were mostly running tasks that needed the repository anyway, but with the introduction of the sparse index, this is no longer the case.

This PR adjusts the code to lazily clone the index when requested, but then also fire off a background thread to start that operation once the background worker has started.

This should allow us to process sparse index updates while the git repository is still being cloned.

Note that due to the way the background worker is implemented this is still not entirely true. While the repository is being cloned a Mutex is being held. At some point all five background worker threads might be handling sync_to_git_index jobs with all of them waiting to lock that mutex. At this point no other jobs can run anymore since the workers are busy. This can be solved in the future by having multiple queues, with a dedicated queue for jobs that involve the git repository.

@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Nov 6, 2023
@Turbo87 Turbo87 requested a review from a team November 6, 2023 22:23
Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The worst case scenario you outlined isn't practically any different to what happens today anyway, so this should be fine.

@bors
Copy link
Contributor

bors commented Nov 7, 2023

☔ The latest upstream changes (presumably #7459) made this pull request unmergeable. Please resolve the merge conflicts.

… background

This commit moves us back to the previous behavior, where the index repository is cloned on startup. But compared to the previous behavior, the cloning is now happening in a background thread so that any background jobs that don't need the repository can run uninterrupted.
@Turbo87 Turbo87 merged commit a8fb349 into rust-lang:main Nov 7, 2023
@Turbo87 Turbo87 deleted the lazy-repo branch November 7, 2023 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants